Skip to content

[seismology] Add DepthLookup abstract interface#199

Open
comoglu wants to merge 4 commits into
SeisComP:mainfrom
comoglu:feature/defaultdepthsetter-interface
Open

[seismology] Add DepthLookup abstract interface#199
comoglu wants to merge 4 commits into
SeisComP:mainfrom
comoglu:feature/defaultdepthsetter-interface

Conversation

@comoglu

@comoglu comoglu commented May 20, 2026

Copy link
Copy Markdown
Contributor

Summary

Introduces an abstract DepthLookup interface for region- and slab-based
depth lookup in SeisComP processing modules (primarily scautoloc).

Interface (libs/seiscomp/seismology/depthlookup.h):

class DepthLookup : public Core::BaseObject {
  virtual bool   init(const Config::Config &) = 0;
  virtual double fetch(double lat, double lon) const = 0;
  virtual double fetchMaxDepth(double lat, double lon) const = 0;
};
DEFINE_INTERFACE_FACTORY(DepthLookup);
#define REGISTER_DEPTH_LOOKUP(Class, Service) ...

Each backend owns all depth knowledge including fallback; no fallback
parameter on the caller side.

Two built-in backends (depthlookup.cpp):

  • "Constant" — returns depths.constant.value for any location
  • "Polygon" — queries GeoFeatureSet polygons (each must carry a
    defaultDepth attribute); fallback from depths.polygon.fallback;
    regions listed in depths.polygon.regions

A third "Slab2" backend ships in SeisComP/main as the dlslab2
plugin (see SeisComP/main#121).

Test plan

  • Build seiscomp_core — clean compile, no errors
  • Unit test: DepthLookupConstant returns depths.constant.value
  • Unit test: DepthLookupPolygon returns polygon depth inside region, fallback outside

@cla-bot cla-bot Bot added the cla-signed The CLA has been signed by all contributors label May 20, 2026
Comment thread libs/seiscomp/seismology/defaultdepthsetter.cpp Outdated
Comment thread libs/seiscomp/seismology/defaultdepthsetter.cpp Outdated
Comment thread libs/seiscomp/seismology/defaultdepthsetter.h Outdated
@gempa-jabe

Copy link
Copy Markdown
Contributor

I am not in favour of the name DefaultDepthSetter. This is very specific to the intended use-case but it should be a versatile interface to be used by other components as well. As you describe the interface as "depth-lookup" why not calling it DepthLookup or DepthQuery or DepthProvider?. I would appreciate if we could discuss such ideas and names prior to a PR to avoid discussions and additional work later on for you as well.

Comment thread libs/seiscomp/seismology/depthlookup.h Outdated
@comoglu

comoglu commented May 21, 2026

Copy link
Copy Markdown
Contributor Author

I am not in favour of the name DefaultDepthSetter. This is very specific to the intended use-case but it should be a versatile interface to be used by other components as well. As you describe the interface as "depth-lookup" why not calling it DepthLookup or DepthQuery or DepthProvider?. I would appreciate if we could discuss such ideas and names prior to a PR to avoid discussions and additional work later on for you as well.

Yeah you are right. Which one you prefer me to go with?
DepthLookup looks good to me?

@gempa-jabe

Copy link
Copy Markdown
Contributor

Just thinking, if we already have a class doing a similar thing ... Regions comes into my mind, so not very helpful ;)
Yes, DepthLookup sounds reasonable. At least not Setter because it does not set anything, it just replies to a query.

@comoglu comoglu force-pushed the feature/defaultdepthsetter-interface branch from 051c31b to 744ed10 Compare May 21, 2026 10:15
@comoglu comoglu changed the title [seismology] Add DefaultDepthSetter abstract interface [seismology] Add DepthLookup abstract interface May 21, 2026
@comoglu comoglu requested a review from gempa-jabe May 21, 2026 22:13
@comoglu comoglu marked this pull request as ready for review June 4, 2026 09:45
comoglu added 3 commits June 4, 2026 22:55
Adds an extensible depth-lookup interface used by scautoloc and other
processing modules to select region- or slab-specific default and maximum
depths instead of a single global value.

Two built-in implementations: "Constant" (passthrough) and "Polygon"
(named GeoFeatureSet regions with defaultDepth/maxDepth attributes).
Third-party backends register via REGISTER_DEPTH_LOOKUP in a plugin.
Rename getDefaultDepth/getMaxDepth → fetch/fetchMaxDepth; remove
caller-supplied fallback parameter. Each backend now owns its full
depth knowledge including fallback via its own config namespace:
  depths.constant.value, depths.polygon.regions,
  depths.polygon.fallback.
Per Jan's review: fetch() and fetchMaxDepth() now throw std::out_of_range
when no region/zone contains the given location. Callers that need a
fallback value use the utility functions fetchDepth() and fetchMaxDepth()
which catch and return the fallback.

Removes the fallback ownership from DepthLookupPolygon — callers decide
what to do when outside all configured regions.
@comoglu comoglu force-pushed the feature/defaultdepthsetter-interface branch from a886635 to eff40d7 Compare June 4, 2026 12:56
@comoglu

comoglu commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Addressed Jan's review feedback:

  • Class name: renamed DefaultDepthSetterDepthLookup (done in previous commit)
  • Copyright / code style: old defaultdepthsetter.* files replaced entirely
  • Config namespace: backends use depths.<backend>.* (done in previous commit)
  • Throw-based API (main change in this update): fetch() and fetchMaxDepth() now throw std::out_of_range when no region/zone contains the given location. Two utility free functions fetchDepth() and fetchMaxDepth() are provided for callers that need a fallback value. DepthLookupPolygon no longer owns a fallback or reads depths.polygon.fallback.

@jsaul

jsaul commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Do we have the possibility to retrieve more than one depth? I am referring to the situation where there is deep seismicity along a subducting slab, but also shallow seismicity right above it. That separation can be significant and we need to be careful not to trust a potentially wrong depth.

@gempa-jabe

Copy link
Copy Markdown
Contributor

Thank you for the changes. What I still do not like very much are the utility functions. I proposed that initially to provide a fallback rather than incorporating it into the fetch methods. The better design worked out later was that the client (scautoloc) does not need to provide a default depth. This is currently required as the methods throw exceptions and someone needs to deal with that and provide a fallback. Where does that fallback come from? I prefer that knowledge to be completely encapsulated in the DepthLookup backend and not in the client.

@comoglu

comoglu commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Do we have the possibility to retrieve more than one depth? I am referring to the situation where there is deep seismicity along a subducting slab, but also shallow seismicity right above it. That separation can be significant and we need to be careful not to trust a potentially wrong depth.

Good point , the current interface only returns a single depth. Are you thinking the interface should return a list of candidate depths for scautoloc to try, or more that scautoloc itself should decide to try both a shallow and a slab-based seed independently?

@comoglu

comoglu commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the changes. What I still do not like very much are the utility functions. I proposed that initially to provide a fallback rather than incorporating it into the fetch methods. The better design worked out later was that the client (scautoloc) does not need to provide a default depth. This is currently required as the methods throw exceptions and someone needs to deal with that and provide a fallback. Where does that fallback come from? I prefer that knowledge to be completely encapsulated in the DepthLookup backend and not in the client.

Understood, fetch() should always return a value, with the fallback knowledge fully encapsulated in the backend. The client should never need to handle an exception or supply a depth. Will remove the utility functions and have each backend return its own configured fallback when no zone matches.

@gempa-jabe

gempa-jabe commented Jun 4, 2026

Copy link
Copy Markdown
Contributor

Why do I have the feeling chatting with Claude AI rather than yourself? ;)

@comoglu

comoglu commented Jun 4, 2026 via email

Copy link
Copy Markdown
Contributor Author

Reworked based on Jan's feedback: fetch() and fetchMaxDepth() always
return a value, fallback is configured inside the backend. Removed
the utility functions that required the client to supply a fallback.
@comoglu

comoglu commented Jun 4, 2026

Copy link
Copy Markdown
Contributor Author

Thank you for the changes. What I still do not like very much are the utility functions. I proposed that initially to provide a fallback rather than incorporating it into the fetch methods. The better design worked out later was that the client (scautoloc) does not need to provide a default depth. This is currently required as the methods throw exceptions and someone needs to deal with that and provide a fallback. Where does that fallback come from? I prefer that knowledge to be completely encapsulated in the DepthLookup backend and not in the client.

Done, removed the utility functions and the fallback is back where it belongs, inside the backend. fetch() and fetchMaxDepth() always return a value now, the client doesn't know or care about fallbacks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla-signed The CLA has been signed by all contributors

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants